Sync registered model aliases via SetAlias/DeleteAlias in direct mode by varundeepsaini · Pull Request #4637 · databricks/cli

varundeepsaini · 2026-03-03T11:00:01Z

Summary

The Update API ignores the aliases field on registered models, so aliases defined in bundle config were silently not applied in direct deployment mode.
Sync aliases after create/update using explicit SetAlias/DeleteAlias API calls.
Enable IncludeAliases on read so drift detection picks up alias changes.

Test plan

Unit tests for create with aliases, update add/modify/delete aliases, remove all aliases, and no-op when unchanged
Existing TestAll/registered_models CRUD test passes

denik · 2026-03-03T11:33:44Z

bundle/direct/dresources/registered_model.go


+	// The Create API does not apply aliases, so we sync them separately.
+	if err := r.syncAliases(ctx, response.FullName, config.Aliases, nil); err != nil {
+		return "", nil, fmt.Errorf("failed to sync aliases: %w", err)


Do we need to wrap the error? Our errors already printed with context such as API endpoint.

denik · 2026-03-03T11:35:14Z

bundle/direct/dresources/registered_model_test.go

+	remote, err := r.DoRead(ctx, id)
+	require.NoError(t, err)
+
+	assert.Len(t, remote.Aliases, 2)


Can these be rewritten as acceptance test? You can capture requests there, run them against cloud, and also read remote state with databricks CLI.

denik · 2026-03-03T11:37:34Z

bundle/direct/dresources/registered_model.go


 func (r *ResourceRegisteredModel) DoUpdate(ctx context.Context, id string, config *catalog.CreateRegisteredModelRequest, _ Changes) (*catalog.RegisteredModelInfo, error) {
+	// Fetch current remote state to determine which aliases to add/remove.
+	remote, err := r.client.RegisteredModels.Get(ctx, catalog.GetRegisteredModelRequest{


As a general guideline, DoUpdate should not read remote state. We already have it in the framework and we can pass it if it's needed. However, in this case we can probably get by reading passed Changes?

simonfaltum

Review (automated, 2 agents)

[Critical] Alias ordering causes infinite drift (never converges)

bundle/direct/dresources/registered_model.go (syncAliases)

syncAliases converts slices to maps, destroying order. RemapState returns aliases as-is from the backend. Without KeyedSlices() for aliases, the planner compares by index. If the backend returns aliases in a different order, direct mode plans updates forever. The acceptance test sorts aliases with sort_by(.alias_name), masking this.

Suggestion: Implement KeyedSlices() keyed by alias_name, or sort aliases consistently in RemapState. Add an idempotence test (unchanged redeploy is a no-op).

[Major] DoCreate/DoUpdate return stale state (aliases missing)

bundle/direct/dresources/registered_model.go:72,112

Both methods return the API response captured before syncAliases runs. The Create API doesn't apply aliases and the Update request sets Aliases: nil, so saved deployment state will have aliases: null. System self-heals on next deploy via DoRead, but bundle summary will show stale data.

Suggestion: After syncAliases, do a final DoRead, or merge aliases into the response before returning.

[Major] syncAliases failure after model mutation leaves diverged state

bundle/direct/dresources/registered_model.go:68-70,109-112

If SetAlias/DeleteAlias fails after the model is already created/updated, the workspace is mutated but the state file is unchanged. On create, this leaves an untracked registered model.

Suggestion: Move alias reconciliation to a post-save hook, or persist state before running alias side effects.

[Gap (Major)] No test for DoUpdate with Changes parameter

bundle/direct/dresources/registered_model_test.go

The primary production update path (changes.HasChange(pathAliases)) is not tested. Tests exercise syncAliases directly but not the Changes gating logic.

[Gap (Major)] No idempotence test (unchanged redeploy)

acceptance/bundle/resources/registered_models/aliases/script

Test only redeploys after config changes. Never verifies an unchanged deploy is a no-op.

[Nit] Unnecessary remote fetch in DoCreate

Model was just created, aliases are guaranteed empty. Pass empty slice instead of nil to avoid wasted API call.

[Nit] pathAliases variable style

Use var (...) block with descriptive comment, matching model_serving_endpoint.go pattern.

[Suggestion] Error wrapping in syncAliases

Wrap errors with context: fmt.Errorf("syncing aliases for %s: %w", fullName, err)

[Suggestion] Unnecessary remote fetch in DoCreate

Pass []catalog.RegisteredModelAlias{} instead of nil to skip the GET call.

varundeepsaini · 2026-03-12T14:24:26Z

@simonfaltum

have made the changes had a few thoughts on

Stale state from DoCreate/DoUpdate: we save the local config (newState), not the API response . So missing aliases in the response don't affect state or bundle summary. The return value is only used for cross-resource refs, and nothing references registered_model.aliases.
it would be corrrect to return the latest data but that would add a API call for no reason (as nothing is using it), do i stiill add it ?

Signed-off-by: Varun Deep Saini <varun.23bcs10048@ms.sst.scaler.com>

github-actions · 2026-03-12T14:29:28Z

An authorized user can trigger integration tests manually by following the instructions below:

Trigger:
go/deco-tests-run/cli

Inputs:

PR number: 4637
Commit SHA: 4256d018d8b7433299c88996b0f6c09067f99909

Checks will be approved automatically on success.

simonfaltum · 2026-03-12T14:32:22Z

No, don't add the extra API call. You're right that nothing references registered_model.aliases from the return value, and the framework persists local config (not remote state) anyway. The next plan cycle will fetch fresh state via DoRead. An extra GET call here would just add latency for no benefit. What you have is fine.

simonfaltum · 2026-03-12T14:32:59Z

@denik Would you do the final approval since you did the original review?

varundeepsaini requested review from andrewnester, anton-107, denik, pietern and shreyas-goenka as code owners March 3, 2026 11:00

varundeepsaini force-pushed the fix-registered-model-aliases-direct branch from ab92b8b to 034aac0 Compare March 3, 2026 11:09

denik reviewed Mar 3, 2026

View reviewed changes

varundeepsaini force-pushed the fix-registered-model-aliases-direct branch from 034aac0 to cf22e16 Compare March 3, 2026 12:57

varundeepsaini requested a review from simonfaltum as a code owner March 3, 2026 12:57

varundeepsaini force-pushed the fix-registered-model-aliases-direct branch from cf22e16 to 86d4e85 Compare March 3, 2026 12:58

varundeepsaini requested a review from denik March 3, 2026 12:58

varundeepsaini force-pushed the fix-registered-model-aliases-direct branch from 86d4e85 to 1300e29 Compare March 3, 2026 16:52

simonfaltum reviewed Mar 12, 2026

View reviewed changes

varundeepsaini force-pushed the fix-registered-model-aliases-direct branch from 1300e29 to d04fd1f Compare March 12, 2026 14:23

varundeepsaini force-pushed the fix-registered-model-aliases-direct branch from d04fd1f to d08815a Compare March 12, 2026 14:27

varundeepsaini requested a review from simonfaltum March 12, 2026 14:27

Sync registered model aliases via SetAlias/DeleteAlias in direct mode

4256d01

Signed-off-by: Varun Deep Saini <varun.23bcs10048@ms.sst.scaler.com>

varundeepsaini force-pushed the fix-registered-model-aliases-direct branch from d08815a to 4256d01 Compare March 12, 2026 14:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sync registered model aliases via SetAlias/DeleteAlias in direct mode#4637

Sync registered model aliases via SetAlias/DeleteAlias in direct mode#4637
varundeepsaini wants to merge 1 commit intodatabricks:mainfrom
varundeepsaini:fix-registered-model-aliases-direct

varundeepsaini commented Mar 3, 2026

Uh oh!

denik Mar 3, 2026

Uh oh!

denik Mar 3, 2026

Uh oh!

denik Mar 3, 2026 •

edited

Loading

Uh oh!

simonfaltum left a comment

Uh oh!

varundeepsaini commented Mar 12, 2026

Uh oh!

github-actions bot commented Mar 12, 2026

Uh oh!

simonfaltum commented Mar 12, 2026

Uh oh!

simonfaltum commented Mar 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

varundeepsaini commented Mar 3, 2026

Summary

Test plan

Uh oh!

denik Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

denik Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

denik Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simonfaltum left a comment

Choose a reason for hiding this comment

Review (automated, 2 agents)

[Critical] Alias ordering causes infinite drift (never converges)

[Major] DoCreate/DoUpdate return stale state (aliases missing)

[Major] syncAliases failure after model mutation leaves diverged state

[Gap (Major)] No test for DoUpdate with Changes parameter

[Gap (Major)] No idempotence test (unchanged redeploy)

[Nit] Unnecessary remote fetch in DoCreate

[Nit] pathAliases variable style

[Suggestion] Error wrapping in syncAliases

[Suggestion] Unnecessary remote fetch in DoCreate

Uh oh!

varundeepsaini commented Mar 12, 2026

Uh oh!

github-actions bot commented Mar 12, 2026

Uh oh!

simonfaltum commented Mar 12, 2026

Uh oh!

simonfaltum commented Mar 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

denik Mar 3, 2026 •

edited

Loading